Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show warning message when last user input get pruned #4816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jazzcort
Copy link
Contributor

@Jazzcort Jazzcort commented Mar 25, 2025

Description

If the user's last input is pruned due to context overflow, a warning message will be displayed in the chat section, alerting them that some details may have been lost. As a result, the response they receive might be incomplete or inaccurate due to the truncated input.
Granite-Code/granite-code#22

Screenshots

show-warning-in-chat

Testing instructions

Set the model’s context length to a small value (e.g., 512) and ask a question that exceeds contextLength - maxTokens tokenwise. A warning message will appear at the bottom of the chat section, indicating that some input may have been truncated. Deleting previous messages will remove the warning.

@Jazzcort Jazzcort requested a review from a team as a code owner March 25, 2025 18:21
@Jazzcort Jazzcort requested review from RomneyDa and removed request for a team March 25, 2025 18:21
Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit f97736e
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67f557b47a4b61000849ebbd

Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jazzcort this could be a great addition, could you explore solutions that avoid injecting a new warning message type into the chat messages, along with a more subtle warning UI?

@Jazzcort
Copy link
Contributor Author

@RomneyDa I'll try to find another way to send the warning message back to the webview.

@RomneyDa
Copy link
Collaborator

RomneyDa commented Mar 28, 2025

@Jazzcort I'd be interested in having this sort of "stream warning" idea as well so fell free to bounce approach/ideas here before spending too much time on them! I think there could be several different approaches where the warnings aren't persisted to chat history, maybe passing them with streams but with a "warning:" field that is captured in redux streamUpdate and temporarily added to UI or something. Let me know thoughts

@Jazzcort
Copy link
Contributor Author

I've already implemented this second approach in the following branch: Jazzcort/warn-when-truncate-last-msg-v2.

Instead of sending the warning through the stream, I used the messenger system to deliver the warning message. With this approach, I make the pruning behavior occur before calling streamChat so I can reach the messenger reference. The advantage of this approach is that users receive the warning message before the streamed response rather than after it has finished.

Regarding the "warning:" field, are you suggesting adding it to AssistantChatMessage? I think that could work as well! I'm open to either approach—whichever aligns better with the project's design. We can also discuss the UI implementation afterward.

@owtaylor
Copy link
Contributor

Another idea would be to extend @Jazzcort's last approach and have two separate calls from webview => core.

  1. llm/pruneChat => {prunedMessages, warning?: string}
  2. llm/streamChat

or something like that. (llm.streamChat could still prune itself when it's not being invoked from the chatview.)

This would avoid having to worry about the interaction between warnings and streaming. It could also be potentially useful for some other things:

  • prefix-caching sensitive pruning (project writeup: Context stability - taking advantage of prefix caching Granite-Code/granite-code#96)
  • having some way to reveal the pruned messages in the UI. I'm not at all sure that this is a good idea - the user can look at logs - but I do feel that it can be deceptive to have a rich chat history with no indication that only a tiny fraction of it might be actually be sent to the model.

@Jazzcort
Copy link
Contributor Author

I agree with @owtaylor's suggestion. Calling llm/pruneChat before llm/streamChat not only helps manage context length but also provides control over whether llm/streamChat is called at all. Users might not focus too much on the last response when they see the warning message.

Additionally, leveraging Context stability - taking advantage of prefix caching is a great strategy. It can enhance the user experience by reducing response time when the context limit is reached. @RomneyDa If everything sounds great, I'll start working on it.

@RomneyDa
Copy link
Collaborator

RomneyDa commented Apr 1, 2025

Planning to look into this tomorrow midday!

@RomneyDa
Copy link
Collaborator

RomneyDa commented Apr 2, 2025

@Jazzcort @owtaylor would agree that two separate calls is a good approach, especially having the warning up front would be great and it won't effect all the other uses of streamChat in core, etc. llm/pruneChat could work, I'd be interested in approaches to this that don't do the pruning twice, but also use the same streamChat function. Perhaps some kind of alreadyPruned boolean passed to llm.streamChat. Counting tokens can be a bit expensive but not bad.

@sestinj tagging since touches core streaming

@Jazzcort
Copy link
Contributor Author

Jazzcort commented Apr 3, 2025

I'm planing to implement llm/compileChat which call compileChatHistory to prune the chat messages we pass and return the compiled chat messages and a boolean that indicates whether we should warn users or not. I'll also add a boolean parameter to llm/streamChat so we won't do the pruning twice. How do you guys think? @owtaylor @RomneyDa @sestinj

@Jazzcort Jazzcort force-pushed the Jazzcort/warn-when-truncate-last-msg branch 4 times, most recently from 50920c6 to 495ae05 Compare April 7, 2025 18:59
Copy link
Contributor

@owtaylor owtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me. Just two suggestions.

core/core.ts Outdated

if (completionOptions.model === "o1") {
completionOptions.stream = false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, duplicating this type of logic into two places is going to make things difficult to maintain in the future. I would suggest making model._compileChatMessages() public [and not _-prefixed] and using that. You could make it call this._modifyCompletionOptions() itself as well, since calling that multiple times shouldn't hurt.

if (lastMessageTruncated) {
dispatch(
setWarningMessage(
"The context has reached its limit. This may lead to less accurate answers.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the context has reached its limit" sounds like the entire chat is too long. Maybe:

The provided context items are too large. They have been truncated to fit within the model's context length.

It's a bit verbose but:

  • Say more specifically what happened
  • Contain the phrase "context length" so that if the user searches they can find out what the context length is.

@Jazzcort Jazzcort force-pushed the Jazzcort/warn-when-truncate-last-msg branch from 495ae05 to 173c73a Compare April 7, 2025 19:42
core/core.ts Outdated
const model = await this.configHandler.llmFromTitle(modelName);

options.log = undefined;
const completionOptions: CompletionOptions = mergeJson(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, thinking about it, it's probably better to have a public llm.compileChatMessages() that takes the LLMFullCompletionOptions, and have that and streamChat() call _compileChatMessages() - sorry for not suggesting that on the previous round.

@Jazzcort Jazzcort force-pushed the Jazzcort/warn-when-truncate-last-msg branch from 173c73a to 064164a Compare April 7, 2025 20:43
Copy link
Contributor

@owtaylor owtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me. @RomneyDa - do you still think the warning needs to be more subtle? From my perspective, when the last message is truncated, the results are unlikely to be good.

@Patrick-Erichsen
Copy link
Collaborator

Patrick-Erichsen commented Apr 8, 2025

Not to make this a design by committee, but my only feedback is that I think the warning should be yellow rather than red. Red feels a bit too scary for the severity of this warning imo.

Great contribution though, thanks @Jazzcort ! 👌

First, I integrated llm/compileChat to precompile chat messages before
invoking llm/streamChat. The llm/compileChat function returns both the
compiled messages and a boolean flag indicating whether the most
recent user input has been pruned. This allows us to trigger a warning
to notify users when pruning occurs at the last message.

The messageOptions is introduced to streamChat as an optional parameter.
If we pass messageOptions and the precompiled attribute is set to true,
streamChat will skip the process of compiling chat messages, ensuring
that the messages won't go through the pruning process twice.
@Jazzcort Jazzcort force-pushed the Jazzcort/warn-when-truncate-last-msg branch from 064164a to f97736e Compare April 8, 2025 17:06
@Jazzcort
Copy link
Contributor Author

Jazzcort commented Apr 8, 2025

@Patrick-Erichsen I’ve updated the warning message color to yellow—thanks for the feedback!
@RomneyDa Let me know if you'd like to tweak the UI further. We can either make those changes now or go ahead and merge this PR and revisit the design later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants